CLOUDP-304929: xgen-IPA-102-collection-identifier-camelCase #499

wtrocki · 2025-03-10T16:14:14Z

Collection Identifier camelCase Rule (xgen-IPA-102-collection-identifier-camelCase):

Enforces camelCase naming convention for resource identifiers
Prevents use of kebab-case, snake_case, or PascalCase
Also validates custom method names in the path

wtrocki · 2025-03-10T20:12:04Z

Will apply approach for each path based on @lovisaberggren comment and also style from refactor PR. Back to draft

tools/spectral/ipa/__tests__/collectionIdentifierCamelCase.test.js

lovisaberggren · 2025-03-11T09:54:34Z

tools/spectral/ipa/rulesets/functions/utils/componentUtils.js

+ * @param {string} str - The string to check
+ * @returns {boolean} - True if the string is in camelCase, false otherwise
+ */
+export function isCamelCase(str) {


FYI there is a spectral function we can use to check camel case
import { casing } from '@stoplight/spectral-functions'; like here https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustUseCamelCase.js#L3

I have considered it but preference is to have casing defined. Will change it now.

Please use the spectral function, see

openapi/tools/spectral/.spectral.yaml

Lines 27 to 29 in 698e772

function: pattern # casing?

functionOptions:

match: "/^[a-z$_]{1}[A-Z09$_]*/" # type: camel ?

where the current regex is broken and could've been a built-in spectral check

also

openapi/tools/spectral/.spectral.yaml

Lines 37 to 39 in 698e772

function: pattern # casing?

functionOptions:

match: "^[A-Z].*" # type: pascal ?

Do we still use this helper function? We can remove it if we are going with Spectral function

Removing helper.

yelizhenden-mdb · 2025-03-11T12:01:17Z

tools/spectral/ipa/rulesets/functions/collectionIdentifierCamelCase.js

+
+function checkViolations(pathKey, path) {
+  const violations = [];
+  const pathSegments = pathKey.split('/').filter((segment) => segment.length > 0);


We might need to slice the prefix before looking at collectionIdentifiers
(Similar to here

openapi/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js

Line 34 in e237b66

const prefix = getPrefix(input);

)

I would go with ignoring some specific use cases rather than having prefix.
In this case we need v2 to be ignored.

WDYT?

Done. Used spectral native way to configure values.
Will fix eachPathAlternatesBetweenResourceNameAndPathParam.js file once we review this PR and approve it.

Will update our spec to avoid hardcoded values that depend on downstream schema.

yelizhenden-mdb · 2025-03-11T12:03:40Z

tools/spectral/ipa/rulesets/functions/collectionIdentifierCamelCase.js

+    if (!segment) return;
+
+    // Skip path parameter validation if it matches the expected format
+    if (isPathParam(segment)) {


I suppose we are not skipping pathParams, instead we are still validating them. This rule should look for collectionIdentifiers only

not skipping

We do not have rule to cover validation for path param

collectionIdentifiers only

Currently we do have those as part of the collection identifiers. e.g

paths./api/atlas/v2/groups/{groupId}/clusters/{clusterName}/globalWrites/managedNamespaces.delete

I did checked briefly and could not find an rule to validate camel case for path params.
No strong opinion on my side. I have documented this rule to cover path params as well but can change it.

I'm leaning towards checking for more use cases (and documenting those).
Leaving with no action however happy to change it if we do have specific validation for this already or based on your feedback and opinion.

Yes, there is no guideline about path param (i.e. resourceID) format in IPA-102. Maybe we can consider validating it after defining a guideline about resourceID format and then validating with a rule that just looks for that guideline. Wdyt?

Discussed offline. We need to update rule description and name to reflect that it validates whole path correctness.
Then we can do all validation here. Thank you @yelizhenden-mdb

yelizhenden-mdb · 2025-03-11T12:07:05Z

tools/spectral/ipa/rulesets/functions/collectionIdentifierCamelCase.js

+    }
+
+    // Skip empty identifiers
+    if (identifier.length === 0) {


I could not think of any empty identifier case? Do we expect such cases like /api/atlas/v2/resource/:customMethod

Do we expect such cases like /api/atlas/v2/resource/:customMethod

My approach is to cover all possible cases and have clear handling for those.
Since you made comment I think we can have it as error use case?
WDYT?

Sure, we can have it as test case

Awesome. Adding extra details and tests.

wtrocki · 2025-03-11T16:20:29Z

Noticed I missed test for //. Will add it in follow up PR

lovisaberggren

LGTM, some nits and one Q

tools/spectral/ipa/rulesets/IPA-102.yaml

tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js

lovisaberggren · 2025-03-11T16:25:21Z

tools/spectral/ipa/__tests__/collectionIdentifierCamelCase.test.js

+      paths: {
+        '/resource_groups': {},
+      },
+    },
+    errors: [
+      {
+        code: 'xgen-IPA-102-collection-identifier-camelCase',
+        message:
+          "Collection identifiers must be in camelCase. Path segment 'resource_groups' in path '/resource_groups' is not in camelCase. http://go/ipa/102",
+        path: ['paths', '/resource_groups'],
+        severity: DiagnosticSeverity.Warning,
+      },


Q: If the path is for example /resource_groups/{id}/resource_keys, will there be two errors, one for resource_groups and one for resource_keys?

Single violation with 2 errors.

Cool, could you add a test case for it?

Extended multiple test cases.

Co-authored-by: Lovisa Berggren <[email protected]>

lovisaberggren

TY!

wtrocki changed the base branch from main to CLOUDP-304930 March 10, 2025 16:14

wtrocki force-pushed the CLOUDP-304930 branch 3 times, most recently from d82590d to a8e254c Compare March 10, 2025 16:44

wtrocki changed the base branch from CLOUDP-304930 to main March 10, 2025 16:45

wtrocki force-pushed the CLOUDP-304929 branch 2 times, most recently from b4350a1 to e5e3d89 Compare March 10, 2025 20:00

wtrocki changed the title ~~Cloudp 304929~~ @wtrocki CLOUDP-304929: xgen-IPA-102-collection-identifier-camelCase Mar 10, 2025

wtrocki changed the title ~~@wtrocki CLOUDP-304929: xgen-IPA-102-collection-identifier-camelCase~~ CLOUDP-304929: xgen-IPA-102-collection-identifier-camelCase Mar 10, 2025

wtrocki force-pushed the CLOUDP-304929 branch from e5e3d89 to e89c0fd Compare March 10, 2025 20:01

wtrocki marked this pull request as ready for review March 10, 2025 20:01

wtrocki requested a review from a team as a code owner March 10, 2025 20:01

wtrocki marked this pull request as draft March 10, 2025 20:11

wtrocki force-pushed the CLOUDP-304929 branch from e89c0fd to d9d45d4 Compare March 11, 2025 09:14

wtrocki marked this pull request as ready for review March 11, 2025 09:16

lovisaberggren reviewed Mar 11, 2025

View reviewed changes

wtrocki force-pushed the CLOUDP-304929 branch from d9d45d4 to 7918594 Compare March 11, 2025 10:22

wtrocki requested a review from lovisaberggren March 11, 2025 11:51

wtrocki force-pushed the CLOUDP-304929 branch from e65dfe3 to 0e2d4c4 Compare March 11, 2025 11:53

yelizhenden-mdb reviewed Mar 11, 2025

View reviewed changes

wtrocki force-pushed the CLOUDP-304929 branch from e8e3077 to 5fe67c9 Compare March 11, 2025 13:49

wtrocki added 5 commits March 11, 2025 16:42

CLOUDP-304929: camel case

60308e4

fix: reuse method

a9152e2

fix: remove function

d4ce211

fix: review comments

7751d68

fix: example description

f36dce5

wtrocki force-pushed the CLOUDP-304929 branch from 5fe67c9 to f36dce5 Compare March 11, 2025 16:08

lovisaberggren approved these changes Mar 11, 2025

View reviewed changes

Apply suggestions from Lovisa code review

c4f3b26

Co-authored-by: Lovisa Berggren <[email protected]>

wtrocki requested a review from lovisaberggren March 11, 2025 17:16

fix: extra unit tests and dedicated handling for slashes in input

7459c18

wtrocki force-pushed the CLOUDP-304929 branch from d9af668 to 7459c18 Compare March 11, 2025 17:17

lovisaberggren approved these changes Mar 11, 2025

View reviewed changes

wtrocki merged commit 246df4d into main Mar 11, 2025
13 checks passed

wtrocki deleted the CLOUDP-304929 branch March 11, 2025 17:52

	function: pattern # casing?
	functionOptions:
	match: "/^[a-z$_]{1}[A-Z09$_]*/" # type: camel ?

	function: pattern # casing?
	functionOptions:
	match: "^[A-Z].*" # type: pascal ?

Uh oh!

CLOUDP-304929: xgen-IPA-102-collection-identifier-camelCase #499

CLOUDP-304929: xgen-IPA-102-collection-identifier-camelCase #499

Uh oh!

Conversation

wtrocki commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtrocki commented Mar 10, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtrocki Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtrocki commented Mar 11, 2025

Uh oh!

lovisaberggren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lovisaberggren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wtrocki commented Mar 10, 2025 •

edited

Loading

wtrocki Mar 11, 2025 •

edited

Loading